GH-50777: [CI][Dev] Fix shellcheck errors in the ci/scripts/r_test.sh - #50783
GH-50777: [CI][Dev] Fix shellcheck errors in the ci/scripts/r_test.sh#50783KHARSHAVARDHAN-eng wants to merge 1 commit into
Conversation
…est.sh ### Rationale for this change This is part of sub-issue apache#44748 to clean up ShellCheck warnings in Apache Arrow shell scripts. * SC1091: Not following: ./bin/activate: openBinaryFile: does not exist (No such file or directory) * SC2086: Double quote to prevent globbing and word splitting. * SC2223: This default assignment may cause DoS due to globbing. Quote it. ### What changes are included in this PR? * SC1091: Add `# shellcheck source=/dev/null` directive. * SC2086: Quote variable expansions (`pushd "${source_dir}"`). * SC2223: Quote default variable assignments (`: "${R_BIN:=R}"`). * Update `.pre-commit-config.yaml` to enable ShellCheck for `ci/scripts/r_test.sh`. ### Are these changes tested? Yes. Executed `shellcheck ci/scripts/r_test.sh` (0 warnings) and `git diff --check` (clean). ### Are there any user-facing changes? No. * GitHub Issue: apache#50777
|
|
|
Hi maintainers, I’ve raised this PR for review. Whenever you have the time, please take a look. If there are any issues, improvements, or changes you’d like me to make, I’d be happy to address them. I’ll be waiting for your feedback. Thank you! |
|
As we've shared on several other issues and PRs, please do not open new PRs until the other PRs have been solved. I am going to close any new PR from you until those are solved. @KHARSHAVARDHAN-eng not taking feedback from other contributors or maintainers into account isn't going to help get these PRs merged. |
|
@raulcd ...Thank you for the feedback. I understand, and I'll focus on addressing the feedback on my existing PRs before opening new ones. Could you also let me know which PRs or review comments you'd like me to prioritize? I want to make sure I'm addressing everything that needs follow-up. Thank you! |
|
You have 4 open PRs right now. One of them on draft, some with comments. Rebase, fix those, reply to comments and engage with the project in order to have your first PR merged. You've opened 12 PRs in one week and 8 of them have been closed due to them not being correct, being opened while other PRs have already solved the problems or had commits that were not related to them. I'd personally would focus on a single PR, the one that feels more interesting to you, and try to get that one merged, as shared on another comment we appreciate quality over quantity. |
2 participants
Rationale for this change
This is part of sub-issue #44748 to clean up ShellCheck warnings in Apache Arrow shell scripts.
What changes are included in this PR?
# shellcheck source=/dev/nulldirective.pushd "${source_dir}").: "${R_BIN:=R}")..pre-commit-config.yamlto enable ShellCheck forci/scripts/r_test.sh.Are these changes tested?
Yes. Executed:
shellcheck ci/scripts/r_test.sh(0 warnings)git diff --check(clean)Are there any user-facing changes?
No.
Closes #50777